[blazor] fix multiple calls to OnConnectionDownAsync #62518
Conversation
| if (this.Client.Connected) | ||
| { | ||
| await OnConnectionDownAsync(CancellationToken.None); | ||
| } |
There was a problem hiding this comment.
I don't think this is the right change to fix the metric issue. It's changing an existing behavior of the system that users might be relying on.
There was a problem hiding this comment.
this event is called twice. That's a bug not a feature
There was a problem hiding this comment.
What is causing the 2nd call?
There was a problem hiding this comment.
The proper event is Hub.OnDisconnect()
CircuitHost.OnConnectionDownAsync() is called from CircuitHost.DisposeAsync() regardless if it's connected or not, creating dis-balance.
There was a problem hiding this comment.
@javiercn does this make sense or you have another proposal for the fix ?
There was a problem hiding this comment.
I just tested the "close tab" and "navigate to non-interactive page". Both behave as expected, that is, they call the event once.
There was a problem hiding this comment.
Terminate it's not being fired in those cases.
- Close tab will likely don't cause
unloadto fire. navigate to non-interactive page- It takes a bit of time for the client to start the circuit disposal.
- If for some reason it's not happening, it's also a bug.
If we check for the circuit to be connected it won't fire when we want to eagerly terminate the circuit.
There was a problem hiding this comment.
Maybe we need to call that event from places that call circuitHost.Client.SetDisconnected() rather than relying that circuitHost.DisposeAsync() would always call it.
There was a problem hiding this comment.
Also I'm not sure that the new PauseAndDisposeCircuitHost is calling SetDisconnected()
There was a problem hiding this comment.
I made the calls to OnConnectionDownAsync more explicit now, instead of relying on the DisposeAsync.
@javiercn please review again.
dotnet-policy-service
bot
added
the
pending-ci-rerun
pavelsavara
removed
the
pending-ci-rerun
CircuitHost.OnConnectionDownAsync()fromCircuitHost.DisposeAsync()if it's not connected_circuitConnectedCounter.AddfromCircuitMetrics.OnCircuitDownFixes #62219